Skip to content

Leader election lease tunables#549

Merged
openshift-merge-bot[bot] merged 1 commit intoopenstack-k8s-operators:mainfrom
hjensas:OSPRH-16335
May 5, 2025
Merged

Leader election lease tunables#549
openshift-merge-bot[bot] merged 1 commit intoopenstack-k8s-operators:mainfrom
hjensas:OSPRH-16335

Conversation

@hjensas
Copy link
Copy Markdown
Contributor

@hjensas hjensas commented May 2, 2025

Expose the leader election tunables for lease duration, renew deadline and retry period via environment variables.

This is inspired by rabbitmq operator exposes the same.

Jira: OSPRH-16335

Depends-On: openstack-k8s-operators/lib-common#627

@openshift-ci openshift-ci bot requested review from frenzyfriday and viroel May 2, 2025 14:36
@openshift-ci openshift-ci bot added the approved label May 2, 2025
Copy link
Copy Markdown
Contributor

@dprince dprince left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. We'll need to decide on timing and roll out similar changes to all service operators.

main.go Outdated
}
}

func getEnvInDuration(envName string) time.Duration {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we add this to lib-common since we want to do this across operators?

main.go Outdated
Comment on lines +118 to +121
if leaseDuration := getEnvInDuration("LEASE_DURATION"); leaseDuration != 0 {
setupLog.Info("manager configured with lease duration", "seconds", int(leaseDuration.Seconds()))
options.LeaseDuration = &leaseDuration
}

if renewDeadline := getEnvInDuration("RENEW_DEADLINE"); renewDeadline != 0 {
setupLog.Info("manager configured with renew deadline", "seconds", int(renewDeadline.Seconds()))
options.RenewDeadline = &renewDeadline
}

if retryPeriod := getEnvInDuration("RETRY_PERIOD"); retryPeriod != 0 {
setupLog.Info("manager configured with retry period", "seconds", int(retryPeriod.Seconds()))
options.RetryPeriod = &retryPeriod
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just thinking about to get this in sync across the operators, could also do this in lib-common and just pass in a ptr to options ? like if we'd then later add another ENV var, we'd just need to add it to lib-common.

@softwarefactory-project-zuul
Copy link
Copy Markdown

This change depends on a change that failed to merge.

Change openstack-k8s-operators/lib-common#627 is needed.

@softwarefactory-project-zuul
Copy link
Copy Markdown

This change depends on a change that failed to merge.

Change openstack-k8s-operators/lib-common#627 is needed.

@hjensas
Copy link
Copy Markdown
Contributor Author

hjensas commented May 5, 2025

/test precommit-check
/test ironic-operator-build-deploy-kuttl

Expose the leader election tunables for lease duration, renew deadline
and retry period via environment variables.

This is inspired by rabbitmq operator exposes the same.

Jira: OSPRH-16335

Depends-On: openstack-k8s-operators/lib-common#627
Copy link
Copy Markdown
Collaborator

@steveb steveb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci bot commented May 5, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hjensas, steveb

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit 2f6d0b9 into openstack-k8s-operators:main May 5, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants